fix(tracing): skip default span attributes when propagating to event #904
Conversation
lcian
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #904 +/- ##
==========================================
- Coverage 73.44% 73.44% -0.01%
==========================================
Files 64 64
Lines 7502 7513 +11
==========================================
+ Hits 5510 5518 +8
- Misses 1992 1995 +3 🚀 New features to boost your workflow:
|
| match &span_data.sentry_span { | ||
| TransactionOrSpan::Span(span) => { | ||
| for (key, value) in span.data().iter() { | ||
| if is_sentry_span_attribute(key) { |
There was a problem hiding this comment.
Is it possible that a user would set a Sentry span attribute manually, and would we want to keep that attribute in that case?
There was a problem hiding this comment.
l: also I would suggest using a .filter on the iterator, rather than the if block with a continue. Imo iterator chains tend to be more readable than branching via if statements
There was a problem hiding this comment.
Unlikely, but in theory it's possible that you could want to override them yeah.
|
This behavior ( |
There was a problem hiding this comment.
Thanks for clarifying, this looks good to me! Would still swap to .filter, but this looks fine to me already
Description
In #887 we added some attributes that are attached by default on all spans to surface more metadata.
We have an option on the
tracinglayer calledenable_span_attributesthat, when emitting an error/log out of atracingevent, attaches all the attributes of each span in the currently active span hierarchy to the error/log.This results in a lot of noise, as you will see those attributes for every span that is currently active, which doesn't provide much value when looking at an error/log.
See this example with a log:
With this change, we'll only propagate user-provided attributes to the error/log, skipping those we set by default on the spans.
When looking at the trace in Sentry, you'll still be able to see those on the corresponding span, as it provides valuable information.
Issues
Close #895
Close RUST-104
#skip-changelog because this fixes a bug for something that wasn't released